Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

util: fix encounter start bug in make/test timeline scripts #14

Closed
wants to merge 3 commits into from
Closed

util: fix encounter start bug in make/test timeline scripts #14

wants to merge 3 commits into from

Conversation

wexxlee
Copy link
Collaborator

@wexxlee wexxlee commented Dec 23, 2023

This PR should address the encounter sync drift referenced in quisquous#5635 and quisquous#6048, as well as the missing zone-seal sync referenced in quisquous#5716.

There were a couple of separate but related issues that I found:

  1. encounter_tools had not (yet) been updated to use InCombat lines to start encounters, even though make_timeline is inserting an InCombat sync at the start of a new timeline.
  2. For fights that do not have zone seals, encounter_tools would fall back on using playerAttackingMob or mobAttackingPlayer regex. While this mostly still worked (with minor drift issues), it was also counting faerie healing actions as the start of the fight. I confirmed this was the case with the log in make/test timeline has weird offset with log file quisquous/cactbot#6048. I don't have logs for the original report from make_timeline/test_timeline drift quisquous/cactbot#5635, but I suspect a similar cause there, as I was unable to repro in e6n when on non-pet classes.
  3. I think there was a minor logic bug in encounter_tools re: pushing the fight-starting log line into logLines. When encountering certain log lines that should trigger a new fight encounter, onStartFight() would reinitialize this.currentFight and set the .startTime property; and when storeStartLine() was subsequently called, it would not push the starting log line into .logLines[] because the fight already had a start time set. This was causing make/test_timeline to sometimes not have access to (and not be able to sync on) the log line that started the encounter.

I'm wary about unintentional breakage, given the various different events that should (or should not) start a timeline. cc: @xiashtra and @JLGarber, would appreciate an extra set of eyes.

@github-actions github-actions bot added docs /docs, /screenshots, *.md resources /resources util /util labels Dec 23, 2023
@wexxlee
Copy link
Collaborator Author

wexxlee commented Dec 23, 2023

git borked, user error. going to close and resubmit.

@wexxlee wexxlee closed this Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs /docs, /screenshots, *.md resources /resources util /util
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant